Add property reflection to notify path and friends calls #5495
Conversation
…losure-compiler renaming. Also quotes the getStyle method of the custom style method since ShadyCSS references the method quoted.
ChadKillingsworth
requested review from
dfreedm,
kevinpschaaf,
sorvell and
TimvdLippe
as code owners
lib/elements/custom-style.js
Outdated
| @@ -75,7 +75,7 @@ export class CustomStyle extends HTMLElement { | |||
| * @export | |||
| * @return {HTMLStyleElement} This element's light-DOM `<style>` | |||
| */ | |||
| getStyle() { | |||
| 'getStyle'() { | |||
There was a problem hiding this comment.
Was this change intentional? It causes an error @export is not supported on this expression.
lib/elements/custom-style.js
Outdated
| @@ -75,7 +75,7 @@ export class CustomStyle extends HTMLElement { | |||
| * @export | |||
| * @return {HTMLStyleElement} This element's light-DOM `<style>` | |||
| */ | |||
| 'getStyle'() { | |||
| ['getStyle']() { | |||
There was a problem hiding this comment.
But why do we need this at all? What's wrong wrong with getStyle() and the @export we already have?
There was a problem hiding this comment.
Good point, export should be sufficient. Will revert back to getStyle()
There was a problem hiding this comment.
@export requires both closure-library and the --generate_exports flag to be used with the compiler making it a not-great solution.
There are two options:
- Remove the
@exportannotation and quote the property directly. - Manually export the method with a
CustomStyle.prototype['getStyle'] = CustomStyle.prototype.getStylestatement right below the class definition.
There was a problem hiding this comment.
All of our internal users have --generate_exports enabled. We're also advocating for it to become enabled by default. So I think that's our preferred solution. Any reason you don't want to turn on the flag?
There was a problem hiding this comment.
Yes it requires closure library. That's a non starter for most external projects.
There was a problem hiding this comment.
Hm, seems odd. I thought it was just a JS compiler pass. Maybe it's like JSCompiler_renameProperty and just requires any implementation of the functions that it generates, since the compiler replaces the call anyway?
There was a problem hiding this comment.
It's very odd. The compiler pass literally adds goog.exportSymbol calls but requires the definition of that method to be included as source. Unlike property reflection, the source actually is present in the final output.
The definition for that method could be injected like a polyfill, but nobody has done that work.
This is a forward port of #5472 and #5491.
There were no linting errors, but that's as much as I can test. I don't currently have a robust project to test Polymer 3 against with closure-compiler renaming.